rpc: resolve eth_feeHistory on the block overlay view - #22987

Pull request overview

This PR fixes an inconsistency in JSON-RPC head resolution during the block-overlay publish/commit window: eth_feeHistory (via the gas oracle) could resolve its head from the committed view while other RPC paths (e.g. eth_blockNumber) report the overlay head, causing fee history windows to lag and intermittently fail RPC tests.

Changes:

• Pin a single overlay-aware temporal read view inside GasPriceOracleBackend so head resolution and per-block sampling are consistent.
• Ensure GasPriceOracleBackend.Fork() uses the same overlay-aware constructor, so fee-history sampling paths that run on the forked tx also see the overlay.
• Add overlay race regression tests covering fee history with and without reward percentiles, and update the test harness to write the forkchoice head marker into the overlay.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Requesting changes. Findings by severity; the first one is reproduced locally on this branch.

High (blocking) — in-flight block receipts silently read as zero

Read views keep DomainReader = sd (db/state/execctx/domain_shared.go:720). The background-commit teardown (execution/execmodule/forkchoice.go:731-736) calls bgSD.Close() → sd.mem.Close() → ClearRam() (db/state/temporal_mem_batch.go:654), which empties the domain maps while RPC views may still hold them. Reproduced: after sd.Close(), a view created earlier returns ok=false, err=nil for an in-flight ReceiptDomain key. The fallback is the reader's own tx — pre-commit for requests started during the publish window — and ReceiptAsOf zero-fills misses (db/rawdb/rawtemporaldb/accessors_receipt.go:42-60). So GetReceiptsGasUsed returns GasUsed=0 for every tx of the head block and the reward percentiles are silently wrong. The window is realistic: a percentile fan-out over many blocks often still runs when the commit finishes.

The class predates this PR (the receipts generator wraps its tx since #22511), but this PR makes the oracle resolve the in-flight head and then sample its receipts, so it leans on the missing lifetime guarantee much harder.

Suggested fix (can be a prerequisite PR in execctx/execmodule): don't clear domain RAM under a published SD — e.g. a teardown variant that closes writer resources but skips ClearRam, leaving the maps to GC once Events and the last view drop the pointer (GetAsOf touches only domains/storage/unwindChangeset, read-only after publication). Refcounting is the heavier alternative.

Medium — Fork can lose the resolved head → whole request fails

Fork opens its tx first and resolves LatestSD() afterwards (via the constructor). If the commit → PublishOverlay(nil) interval lands in between, the forked backend has neither the overlay nor the committed head block. On the header-only path headerByNumber propagates BlockNotFoundErr unfiltered (rpc/jsonrpc/eth_api.go:397-400), so the whole eth_feeHistory request fails with -32000 block not found: N; on the rewards path the head slot is silently dropped (eth_api.go:301-303). Rare in production (the interval includes an fsync'd commit, so the fork goroutine must be descheduled for >1ms between opening and wrapping), but it turns a stale read into a failed request, and it is new with this PR — before, the window never included the in-flight block.

Suggested fix: capture the overlay once in NewGasPriceOracleBackend and let Fork wrap its fresh tx with the parent's overlay instance instead of re-resolving LatestSD(). Plain-table reads on a closed overlay are safe (memStore close is a no-op on the data). Note this makes the high-severity item easier to hit — fork views then always carry the SD's DomainReader — so both fixes should land together.

Low — rewards test never exercises receipts

TestFeeHistory_OverlayHeadWithRewards uses an empty-body overlay block, so it hits the zero-reward guard (rpc/gasprice/feehistory.go:186) and never reads receipt data. Extend it to a block with transactions plus receipt-domain entries and assert non-zero gas — this is the test that pins the high-severity fix.

Low (non-blocking follow-up) — nested overlay views

With b.tx now a view, downstream re-wraps create view-over-view over the same overlay, sharing one *sync.RWMutex: the receipts generator (rpc/jsonrpc/receipts/receipts_generator.go:214,459,691), headerByNumber (rpc/jsonrpc/eth_api.go:407), and fillFeeDefaults receiving the already-wrapped tx (rpc/jsonrpc/eth_fill_transaction.go:118). GetOne/Has hold RLock while falling back to m.db, so nested views take a recursive read lock. Safe today — nothing write-locks a published overlay — but that invariant is enforced nowhere; a writer added later deadlocks. Making WithOverlay/WithTemporalOverlay idempotent would remove this and the redundant double lookups.

High

1. Downstream helpers override the pinned overlay. BaseAPI and receipts helpers re-resolve the live overlay on every call (rpc/rpchelper/helper.go:87, receipts/receipts_generator.go:691, eth_api.go:333) and layer it over the pinned view, so a publish during the request still mixes views. Worst case, after a same-height reorg, ReceiptAsOf reads the new fork's cumulative gas at the old block's txNums (ReceiptDomain is exempt from the inMemHistoryReads guard): wrong GasUsed deltas (uint underflow possible) and reward percentiles from another fork. Suggested fix: make WithOverlay/WithTemporalOverlay no-ops when the passed tx already carries an overlay view. A same-instance check is not enough — it would still wrap pinned view A with live overlay B.

2. Overlay results poison FeeHistoryCache. The cache key is {blockNumber, percentiles} — no hash, no reorg/unwind invalidation (rpc/gasprice/feehistory.go:63). With this PR, "latest" can resolve to a not-yet-committed overlay block. If the background commit fails, or a same-height sibling lands, the dead block's fees keep being served until LRU eviction. Fix: skip caching blocks that come from the overlay (or near head), or add the canonical hash to the key.

Medium

3. Overlay capture and tx open are not atomic. The overlay is captured after the caller has opened the tx (rpc/jsonrpc/eth_system.go:504). If commit + PublishOverlay(nil) land in between, the request sees neither the overlay nor the committed block (stale head). In the opposite direction, the pinned overlay can be one FCU ahead of the tx snapshot, leaving a block visible in neither layer. Capture the overlay atomically with tx acquisition — capture-before-open alone only flips the window.

4. eth_gasPrice still mixes heads. The tip comes from the pinned oracle, but the baseFee addend re-resolves the live overlay (rpc/jsonrpc/eth_system.go:273). One-line fix: read the current header through the pinned b.tx.

5. fillFeeDefaults double-wraps the overlay. FillTransaction/signTransaction wrap the tx (eth_fill_transaction.go:61) and newGasOracle now wraps it again: view-over-view, doubled memTx probes, and diverging heads if the overlay flips between the two wraps. Pass the raw dbTx into newGasOracle, or rely on the no-op rule from point 1.

Low / latent

6. Unenforced write-freeze on the published overlay. All read views share one *sync.RWMutex (newReadViewMut copies the parent's mu) and GetOne holds RLock across the fallback into the inner view — a recursive RLock. This is safe only while nothing writes to a published overlay, and nothing enforces that. Consider a runtime assert (e.g. reject Put after publish/Close). Related: Rollback/Commit nil statelessCursors without taking mu (memory_mutation.go:584), unlike the other writers.

7. Test gap. TestGasPriceOracle_ForkKeepsOverlayAfterUnpublish calls PublishOverlay(nil) but never doms.Close(); production always pairs them. Add doms.Close() before backend.Fork() so the test pins the invariant the PR depends on.

8. Overlay-view tax on committed history. Every gas-oracle read now takes the shared RLock plus a guaranteed memTx miss (doubled by the re-wraps) even when all requested blocks are committed. The no-op rule from point 1 removes the doubling; routing reads below the overlay's base block through the raw tx would remove the rest.

Notes

• Longer term, a per-request pin (e.g. at tx acquisition) would fix this class once — rpc/jsonrpc has ~44 independent overlay re-resolves with the same exposure (eth_getLogs, eth_simulateV1, trace workers).
• Nit: the pin-once rationale is repeated at three sites; one canonical place is enough.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

		if current == overlay || attempt == maxAttempts {
			return tx, current, nil

func NewGasPriceOracleBackend(db kv.TemporalRoDB, tx kv.TemporalTx, baseApi *BaseAPI) *GasPriceOracleBackend {
	return newGasPriceOracleBackendPinned(db, tx, baseApi, baseApi.filters.LatestOverlay())

Combined findings from two independent review passes; the first, second and fourth items were also reproduced with regression tests. Line numbers refer to head 5856794.

High

1. Overlay reorgs defeat the number-keyed fee-history cache. Cacheability is only blockNumber <= committed head (rpc/jsonrpc/eth_system.go:542, rpc/gasprice/feehistory.go:392), but an overlay can replace or unwind blocks at or below that height. Two directions: a cached committed block keeps being served after an overlay sibling replaces the same height (reproduced: the old base fee is returned), and during a commit backlog uncommitted overlay siblings below the limit get cached. Fix: key the cache by block hash (structural — also removes CacheableBlockLimit from OracleBackend), or keep the boundary below the earliest overlay-modified height.

2. FillTransaction builds an oracle with two different overlay pins. The tx is wrapped with overlay A (rpc/jsonrpc/eth_fill_transaction.go:61), but NewGasPriceOracleBackend re-captures LatestOverlay() = B (eth_system.go:506), so b.tx stays on A while Fork wraps goroutine txs with B (eth_system.go:566). Reproduced: parent and fork head hashes differ. Pass the overlay together with the wrapped tx (or let the view expose its overlay identity) so the backend and its forks share one pin.

3. The overlay == nil pin is not sticky. A raw tx carries no CarriesOverlayView marker (eth_system.go:550), so downstream helpers re-resolve the live overlay on every call (rpc/rpchelper/helper.go:87, rpc/jsonrpc/eth_api.go:333, :407). An overlay published mid-request leaks in: eth_gasPrice can sum a tip sampled at head N+1 with a base fee read at head N. Make the nil pin explicit so the whole request stays on one view.

Medium

4. The last beginTxWithOverlay attempt returns the (tx, overlay) pair it has just proven inconsistent (eth_system.go:528), which can produce a gapped view — a block visible in neither the tx snapshot nor the overlay (reproduced: stale head returned). Also, the pointer-equality check treats nil==nil as stable, so a full publish/commit/unpublish cycle between the two captures passes on the first attempt. Return an error (or keep retrying until stable), and compare a monotonic publish sequence number instead of pointer identity — that closes both holes.

5. The "rawTx is the caller's tx before the overlay wrap" contract on CacheableBlockLimit (eth_system.go:542) is unenforced and already broken: the FillTransaction path stores a wrapped view in it. Harmless today (that path never reaches FeeHistory), but on a wrapped tx GetLatestBlockNumber resolves the overlay's uncommitted forkchoice head, so the cache limit would include in-flight blocks. Enforce or assert the contract.

Low / follow-ups

6. Two sources remain unpinned in a request meant to hold one consistent view (both pre-existing): Fork opens fresh base txs whose snapshots can postdate a committed reorg (eth_system.go:558), and the pending block comes from the live mining cache (feehistory.go:230).

7. ~45 call sites keep the non-atomic BeginTemporalRo-then-WithOverlay pattern with the same race window this PR closes for three endpoints — including BlobBaseFee (eth_system.go:350), BaseFee (:377) and BlockNumber (:196) in the same file. Promote beginTxWithOverlay to a shared helper on BaseAPI/Filters; fine as a follow-up.

8. Smaller cleanups: the skip-then-wrap sequence exists in three copies (rpc/rpchelper/filters.go:1177, :1211, eth_system.go:549) — extract one helper. readView duplicates the memDb == nil predicate (db/kv/membatchwithdb/memory_mutation.go:61). CarriesOverlayView (memory_mutation.go:1091) is a concrete-type switch that any future tx wrapper silently defeats — a marker-interface method would fail loudly instead.

9. Nits: drop the call-site sentence ("Only the fee-history path pays for the resolution.") from the CacheableBlockLimit doc comment (eth_system.go:537); remove the Co-Authored-By trailer (e035aa3) and the "Generated with Claude Code" footer from the description — repo convention (CLAUDE.md): don't sign commits or PRs; the description's "stays a draft until that one is merged" sentence is stale now that #23046 is merged.

Pull request overview

Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.

// OverlayViewCarrier is implemented by txs that are pinned overlay views.
// A tx wrapper that embeds such a tx keeps the marker through method
// promotion, where a concrete-type switch would silently lose it.

Combined review (own pass plus verified Copilot/Codex reports), categorized by severity.

High

• rpc/jsonrpc/eth_system.go:528: CanonicalHash delegates to the block reader. In rpcdaemon mode RemoteBlockReader.CanonicalHash ignores b.tx and queries the live service, and the later fetch resolves the number again — a reorg in between caches sibling B's fees under sibling A's hash, and the hash-keyed entry is never invalidated. It also adds one remote call per block even on cache hits. Read kv.HeaderCanonical through the pinned tx, fetch by that exact hash (or validate the fetched header's hash), and fall back to the block reader only for the frozen range. Doing so also removes the new per-block lookup on the warm-cache path and the double hash resolution on misses (rpc/gasprice/feehistory.go:394). (Confirms the unresolved Copilot thread.)

Medium

• rpc/jsonrpc/eth_system.go:354 and :201: eth_baseFee, eth_blobBaseFee, and eth_blockNumber keep the non-atomic BeginTemporalRo → WithTemporalOverlay acquisition — the same race class this PR fixes for the sibling fee endpoints, so the cross-endpoint skew can still fire there. Each is a one-line migration to BeginTemporalRoWithOverlay.
• rpc/jsonrpc/eth_system.go:505: for a tx without a pin, NewGasPriceOracleBackend re-resolves LatestOverlay() with no seq-stability check, so eth_fillTransaction can price maxPriorityFeePerGas and maxFeePerGas on two different heads. Pin at tx acquisition (or reuse the request's captured snapshot) there too.
• db/kv/membatchwithdb/memory_mutation.go:1121: noOverlayView embeds the bare interface, so it (a) drops HasBlockFilesRoTx — every no-overlay request (always, in remote mode) loses the tx-pinned block-files view, paying a view acquisition per read and letting one request straddle a snapshot merge — and (b) promotes Apply, whose callback receives the raw unpinned tx (latent: no rpc/ callers today). Forward BlockFilesRoTx and override Apply as the sibling views do. (Confirms the other unresolved Copilot thread.)
• rpc/rpchelper/filters.go:1227: after 3 acquisition attempts under a moving seq, fee RPCs now fail with errOverlayUnstable — a new client-visible error during FCU catch-up/churn. Prefer serving the last coherent capture (tx plus its captured overlay) over returning an error.

Low

• rpc/rpchelper/filters.go:1214: the (pinned, raw) return contract is a misuse trap — pinned.Rollback() panics on a read view but releases the raw tx on noOverlayView. A single handle whose Rollback releases the raw tx would be safer for the planned follow-up call sites.
• db/kv/membatchwithdb/memory_mutation.go:1087: the OverlayViewCarrier doc overclaims — embedding the tx interface promotes only its static method set, so OverlayView is lost unless the wrapper embeds the concrete view type or forwards it explicitly. Reword.
• rpc/rpchelper/filters.go:1236: LatestOverlay duplicates OverlaySnapshot's resolution chain (via LatestSD and its never-written latestSD fallback); express it as overlay, _ := ff.OverlaySnapshot().
• rpc/jsonrpc/eth_system.go:266 (and the two sibling handlers): newGasOracleFromBackend(NewGasPriceOracleBackend(api.db, tx, api.BaseAPI)) is byte-identical to api.newGasOracle(tx). Relatedly, the backend's overlay field (:498) is derivable from the pinned tx via ViewOverlay(b.tx).
• rpc/gasprice/gasprice_test.go:303: mockOracleBackend.CanonicalHash returns the head hash for every height, violating the ok=false-beyond-head contract and collapsing all cache keys; latent until a test combines the mock with a FeeHistoryCache.
• db/kv/membatchwithdb/memory_mutation.go:1151: the view-of-view branch in newReadViewMut is unreachable (all wrap points guard with CarriesOverlayView); drop it or assert m.overlay == nil.
• Layering: only the OverlayViewCarrier marker needs to live in db/kv/membatchwithdb; PinToOverlay's request-pinning policy could sit next to BeginTemporalRoWithOverlay in rpchelper.

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

func (b *GasPriceOracleBackend) CanonicalHash(ctx context.Context, number uint64) (common.Hash, bool, error) {

Combined review of the current head (e2661e0), categorized by severity. High and Medium gate approval; Low are cheap to fold in; the last section is follow-up material.

High

• rpc/gasprice/feehistory.go:394 — the cache-hit path went from a zero-I/O number-keyed LRU get to a per-block CanonicalHash resolution. In remote rpcdaemon mode a fully-memoized eth_feeHistory(1024, latest) now costs roughly 1024–2048 sequential gRPC round trips (a remote GetOne per block, plus the RemoteBlockReader fallback for pruned heights), and a transient error on this auxiliary lookup fails the whole request via return err — the old hit path had no failure mode at all. Degrade to cacheable=false on a resolution error, and skip the resolution (or keep number-keyed entries) at or below the frozen boundary, where the number-to-hash mapping is immutable.

Medium

• rpc/rpchelper/pinned_tx.go:47 — the view comes from overlay.NewReadView(tx), so the handle's required FreezeInfo() promotes to MemoryMutation.FreezeInfo's panic("not supported") whenever an overlay is published (the no-overlay pin works). Delegate to t.raw.FreezeInfo() (or use NewTemporalReadView, which forwards it) and extend TestBeginTemporalRoWithOverlay_PreservesOptionalInterfaces accordingly. Same class: Apply bypasses the temporal tx's closed-tx guard, and the wrapper hides UnderlyingTx() / Pin() of the wrapped view.
• rpc/jsonrpc/eth_system.go:503 — the fallback pin pairs a caller-opened tx with LatestOverlay() resolved at construction time, without the publish-seq bracket — the torn (tx, overlay) pairing BeginTemporalRoWithOverlay's retry loop exists to close. All production callers pre-pin today, so require the pin (assert CarriesOverlayView) instead of silently re-resolving. Also Fork discards the pinned bool at eth_system.go:518.
• rpc/jsonrpc/overlay_race_test.go:702 — the beginHookDB hooks run testify require and t.Cleanup on errgroup fetcher goroutines (Fork → BeginTemporalRo → hook), violating the FailNow same-goroutine contract: a failing hook Goexits the worker, errgroup.Wait returns nil, and the test fails at an unrelated downstream assert. Return errors from the hooks and assert on the test goroutine; that also lets the five copy-pasted publish/commit/unpublish blocks collapse into one helper.
• rpc/gasprice/gasprice_test.go — mockOracleBackend.CanonicalHash returns m.head.Hash() for every height, so all block numbers share one cache key; dormant only because every NewOracle call passes historyCache=nil. Derive a per-height hash.

Low

• rpc/gasprice/feehistory.go:440 — the compute path re-resolves the canonical hash inside HeaderByNumber/BlockByNumber after line 394 already did, plus a keccak per stored block for the fees.header.Hash() == blockHash guard. Fetching by the already-resolved (hash, number) pair makes the guard hold by construction.
• rpc/jsonrpc/eth_fill_transaction.go:53 — the comment claims the pinned view keeps the whole request on one head, but the GetTransactionCount / EstimateGas sub-calls open their own unpinned txs; scope the comment or thread the pinned tx through.
• The pin-once rationale is written out in full at four sites (eth_system.go:498, eth_system.go:516, rpc/rpchelper/filters.go:1226, memory_mutation.go:1105); keep the canonical statement on PinToOverlay and reduce the rest to pointers. PinnedRoTx.Rollback's comment restates the type doc above it.
• rpc/jsonrpc/eth_system.go:502 — the CarriesOverlayView guard duplicates the same check inside PinToOverlay; newGasOracleFromBackend has a single caller one line above it.
• rpc/jsonrpc/overlay_race_test.go:643 — commitOverlayBlock omits the Execution stage-progress and canonical-TxNums markers production writes, so the harness commit only supports header-only reads; complete the markers or note the limitation before more tests build on it.

Follow-ups (fine as separate issues, not gating this PR)

• Fork (eth_system.go:512) pins the parent's overlay over a freshly opened tx: a reorg of depth two or more committing mid-request can mix two chains in one response (also reachable via the exhausted-attempts fallback in rpc/rpchelper/filters.go:1212). Not a regression — pre-PR Fork had no pin at all.
• The per-endpoint migration leaves ~105 handlers and ~40 WithOverlay call sites on the torn-view pattern; a single acquisition chokepoint (a BaseAPI-level begin helper) would cover all sites at once, after which WithOverlay collapses into PinToOverlay.
• PendingBlockAndReceipts (eth_system.go:579) returns the live pendingBlock(), contradicting the "always a pinned view" field comment — pre-existing; route through the pin or scope the comment.
• A pinned request retains the torn-down SharedDomains (order 10–50MB) for its lifetime — bounded and safe by design, but worth a note in the PR description.

Pull request overview

Copilot reviewed 14 out of 14 changed files in this pull request and generated 1 comment.

Requesting changes for one blocking performance regression.

High

rpc/gasprice/feehistory.go:419: the warm cache path still performs one canonical-hash lookup per hot block. FeeHistory calls localBackend.CanonicalHash before consulting the LRU at line 428 for every block above frozenBound. In rpcdaemon mode this reaches remoteTx.GetOne / SeekExact, so each lookup is a remote stream round trip.

The dual cache regime removes this cost only for old frozen ranges. The recent latest window remains above the frozen boundary, so a fully memoized eth_feeHistory(1024, latest) can still issue up to 1,024 remote lookups on every request. This defeats the cache fast path and creates user-controlled RPC amplification.

Please batch-resolve the hot canonical range or add a reorg-aware number-to-hash cache, and cover the warm rpcdaemon path with a benchmark or regression test.